fix(aqua): strip aqua asset formats from AssetWithoutExt by risu729 · Pull Request #10104 · jdx/mise

risu729 · 2026-05-27T20:21:31Z

Summary

render {{.AssetWithoutExt}} with aqua's full asset suffix list instead of a short hard-coded strip chain
reuse the aqua asset format list used by format detection
add focused aqua-registry coverage for zstd-compressed Windows executable assets

Stack

Stacked on #10404. Target branch remains main.

Context

This reworks #10104 on top of the aqua format-alias cleanup in #10404 after PR 1 (feat(file): support tbz extraction format alias) was merged.

AssetWithoutExt should follow aqua asset filename semantics. The previous implementation stripped common archive suffixes such as .tar.gz, .zip, and .gz, but missed aqua-known suffixes like .zst. That matters for packages such as openai/codex, where the registry can render an asset like codex-x86_64-pc-windows-msvc.exe.zst and use {{.AssetWithoutExt}} as the file source. Without this change, mise looks for the .zst path instead of the decompressed .exe path.

This PR deliberately does not change extraction dispatch, HTTP cache behavior, or unsupported-format policy. Those are split into later PRs in the project plan.

Tests

cargo fmt --check
mise x cargo -- cargo test -p aqua-registry test_aqua_file_src_asset_without_ext_strips_zst
mise x cargo -- cargo test -p aqua-registry test_asset_without_ext_uses_aqua_asset_formats
mise x cargo -- cargo test -p aqua-registry test_format_detects_literal_aqua_aliases
mise x cargo -- cargo test -p aqua-registry test_format_preserves_explicit_aqua_aliases

Summary by CodeRabbit

Refactor
- Improved archive format detection logic for better consistency and maintainability.
Tests
- Added tests for archive format detection and file extension handling.

gemini-code-assist

Code Review

This pull request refactors asset format detection and extension stripping in the aqua-registry crate, and updates the Windows extension completion logic in the aqua backend to ignore version numbers when detecting existing extensions. This prevents issues like duplicate extensions on files containing version dots. Feedback on these changes highlights opportunities to optimize string allocations in asset_without_ext, improve robustness in path_ext_ignoring_version by operating only on the filename rather than the entire path, and remove the redundant aqua_file_src helper function.

greptile-apps · 2026-05-27T20:27:22Z

Greptile Summary

This PR fixes AssetWithoutExt template rendering by replacing a short hard-coded suffix strip chain with a find_map over the shared AQUA_ASSET_FORMATS constant, so all aqua-known archive and compression suffixes (including .zst, .dmg, .pkg, .lz4, etc.) are now stripped correctly.

Extracts AQUA_ASSET_FORMATS as a named const slice and reuses it in both detect_format and the new asset_without_ext helper, eliminating the previous divergence between the two.
The find_map ordering (compound formats before their single-component parts, e.g. tar.gz before gz) ensures multi-part extensions like .tar.gz strip as a unit rather than leaving a trailing .tar.
Adds three focused unit tests covering the motivating .exe.zst case, compound formats, and no-extension pass-through.

Confidence Score: 5/5

Safe to merge — the change is a focused, well-tested refactor of a format-stripping helper with no effect on extraction dispatch or HTTP behaviour.

The new asset_without_ext uses find_map over a shared constant whose ordering correctly places compound suffixes (tar.gz) before their single-component parts (gz), so no existing format is mis-stripped. The motivating .zst bug is fixed and covered by a new test, and the old hard-coded chain's missing formats (.dmg, .pkg, .lz4, etc.) are all included.

No files require special attention.

Important Files Changed

Filename	Overview
crates/aqua-registry/src/types.rs	Extracts AQUA_ASSET_FORMATS constant, replaces hard-coded strip chain with asset_without_ext helper, and adds tests for zst and other previously-unsupported formats. Implementation is logically correct and well-ordered.

_{Reviews (19): Last reviewed commit: "Merge branch 'main' into fix/aqua-window..." | Re-trigger Greptile}

coderabbitai · 2026-05-31T18:04:42Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 48a22ef9-3479-46fa-af9c-3c8f033d310d

📥 Commits

Reviewing files that changed from the base of the PR and between f37e519 and 7edaafc.

📒 Files selected for processing (1)

crates/aqua-registry/src/types.rs

🚧 Files skipped from review as they are similar to previous changes (1)

crates/aqua-registry/src/types.rs

📝 Walkthrough

Walkthrough

This PR centralizes asset format detection and archive extension handling in the aqua-registry. A new shared constant and helper function replace inline suffix matching logic in AquaPackage::detect_format and AquaFile::template_ctx, with supporting unit tests validating the refactored behavior.

Changes

Asset Format and Extension Handling Centralization

Layer / File(s)	Summary
Shared asset format helpers and constants `crates/aqua-registry/src/types.rs`	Introduces `AQUA_ASSET_FORMATS` constant and `asset_without_ext` helper function that centralize logic for stripping known archive extensions (including multi-part tar variants), replacing duplicated inline suffix matching.
Format detection and template context refactoring `crates/aqua-registry/src/types.rs`	Refactors `AquaPackage::detect_format` to iterate over the shared constant list instead of inline hardcoded suffixes, and refactors `AquaFile::template_ctx` to use `asset_without_ext` instead of manually chaining `strip_suffix` calls.
Format handling validation tests `crates/aqua-registry/src/types.rs`	Adds unit tests validating that `.exe` portions are preserved while archive extensions like `.zst` are stripped, and that `asset_without_ext` correctly normalizes various known Aqua archive formats while leaving unknown filenames unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

jdx/mise#10224: Refactors .7z format detection in the same AquaPackage::detect_format method, making both PRs interdependent on archive format matching logic.
jdx/mise#10404: Modifies the same AQUA_ASSET_FORMATS and archive suffix handling in crates/aqua-registry/src/types.rs, refactoring how shorthand formats like tgz and tbz are recognized and treated.

Poem

A rabbit hops through formats neat, 🐰
Where helpers share the work complete—
No more duplication's plight,
The constants glow, the tests shine bright,
Extensions stripped, the code runs light!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(aqua): strip aqua asset formats from AssetWithoutExt' clearly and directly summarizes the main change: refactoring AssetWithoutExt to use aqua's centralized asset format list instead of hardcoded suffixes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/aqua-registry/src/types.rs`:
- Around line 1428-1448: The test
test_remove_ext_from_asset_uses_aqua_asset_formats is missing assertions for the
shorthand tar-based suffixes; update that test to add assertions calling
remove_ext_from_asset for filenames ending with .tbr, .tlz4, and .tsz and assert
they normalize to ("name", "tar.br"), ("name", "tar.lz4"), and ("name",
"tar.zst") respectively (e.g., "tool.tbr" -> ("tool","tar.br"), "tool.tlz4" ->
("tool","tar.lz4"), "tool.tsz" -> ("tool","tar.zst")); keep the test name and
placement and follow the same assert_eq! style as the other cases so the
normalization behavior is covered.
- Around line 891-898: normalize_asset_format currently maps
"tgz","txz","tbz","tbz2" but misses other tar-based shorthands present in
AQUA_ASSET_FORMATS; update the normalize_asset_format function to also map "tbr"
-> "tar.br", "tlz4" -> "tar.lz4", and "tsz" -> "tar.sz" so downstream checks
like format.starts_with("tar") (used in install logic) correctly route these
archives to the untar handler.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 7afc5978-0a73-4902-85d7-6cb6c06db3f0

📥 Commits

Reviewing files that changed from the base of the PR and between ebf4795 and f37e519.

📒 Files selected for processing (2)

crates/aqua-registry/src/types.rs
e2e-win/backend/aqua_bin_path.Tests.ps1

github-actions · 2026-06-06T00:25:53Z

This PR currently has merge conflicts. If this continues for 7 days, it will be closed automatically.

This is warning day 1 of 7.